Add a check for urls in loose mode.#118
Add a check for urls in loose mode.#118robert-j-webb wants to merge 1 commit intoshellscape:masterfrom
Conversation
Fixes shellscape#117 I do not like this solution but I think that it is somewhat close to a good answer.
|
Good first attempt, but the solution should not modifying existing token snapshots. |
Not sure which snapshots you're referring to. I actually made sure that the modifications are improvements over what exists previously, ie: Would you rather have or I think it's pretty straightforward that no one is parsing a URL and they want the punctuation in it to be represented as a colon and an operator. If you mean this one: vs IDK it seems identical to me, in that the main difference is my version has the raws inside of the value, whereas the repo version has them outside. Is this something that people really need to differentiate on? My main use case for this package is prettier and I'm curious as to what other things people are doing with this. |
Except that it's not
Exactly. That's a breaking change and introduces bugs, as raws should not be part of the token value. There's quite a bit to reference as far as how things should look, we've got pretty good tests.
Yes. Never assume that your use case is the only use case. |
| this.current.value === 'url' && this.currToken[0] !== 'string' && | ||
| this.currToken[0] !== ')' && !this.options.loose) { | ||
|
|
||
| this.currToken[0] !== ')' && (checkForUrl(this.currToken[1]) || !this.options.loose)) { |
There was a problem hiding this comment.
should the logic here be to automatically treat url() contents as a string? technically it should follow https://drafts.csswg.org/css-syntax-3/#consume-url-token for an unquoted string, if i'm reading it right (debatable) the alg is just to consume the content unless you hit a bad char or close parens? e.g. a var() in a url shouldn't be treated as an actual var
There was a problem hiding this comment.
agreed. I'm not sure what the spec says (I haven't looked yet) but if you try to use something like this: url(https://foo.bar/?=)bad char); in chrome and firefox, it'll mark it as invalid CSS and won't apply the rule. Even url(https://foo.bar/?=var(baz)bad char); is marked as bad after the closing var paren. I can't tell or not if either browser allow url(https://foo.bar/?=var(bad char); - there's some odd visuals there but neither will apply the rule.
Using the validator: https://jigsaw.w3.org/css-validator/#validate_by_input
fail - url(https://foo.bar/?=var)bad char);
fail - url(https://foo.bar/?=var()bad char);
fail - url(https://foo.bar/?=var(bad char);
So any extra parens when the value is not quoted will result in an error. So we should probably let the parser in place go through the unquoted tokens, and then afterward combine them all into a string. Any invalid unquoted tokens should be picked up. The only gotcha here is that CSS doesn't several unescaped characters in unquoted url params, so we'd have to identify those and run additional checks.
|
Closing as abdoned. If this gets picked back up, please ping me. |
Fixes #117
I do not like this solution but I think that it is somewhat close to a good answer.
This PR contains:
Breaking Changes?
If yes, please describe the breakage.
Not really breakages but it stops parsing splitting up a url into operators. It's just a fixing, although the behavior is different.
Please Describe Your Changes
See #117